Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

markup/goldmark: Add attributes support for blocks (tables etc.) #8215

Merged
merged 1 commit into from
Feb 8, 2021

Conversation

bep
Copy link
Member

@bep bep commented Feb 8, 2021

Fixes #7548

@bep
Copy link
Member Author

bep commented Feb 8, 2021

There are some stuff left here.

This first implementation has the attributes above the block (which also feels natural to me), but looking at what others have done/thinks about this, the common thing is to put it below, e.g.:

> foo
> bar
{.myclass}

vs

{.myclass}
> foo
> bar

@yuin do you have any thoughts on this (also: re. my implementation in this PR)?

/cc @jmooring

@bep bep force-pushed the feat/goldmark-attrs branch from 334369d to fe372e4 Compare February 8, 2021 09:51
@yuin
Copy link

yuin commented Feb 8, 2021

IMHO, There are no standards for attributes. pandoc has the attributes above the block.
Your implementation is no problem. But It may be hard to follow this rule for some elements (i.e. table cells). So attributes for these elements are hard to implement as extensions...

@bep bep force-pushed the feat/goldmark-attrs branch from fe372e4 to 1e557d3 Compare February 8, 2021 10:44
@bep
Copy link
Member Author

bep commented Feb 8, 2021

@yuin thanks, I (and I suspect most people) do not care about table cells ... I think the test cases I've added would make most people happy.

@bep
Copy link
Member Author

bep commented Feb 8, 2021

So, there are more to this syntax than the below, but what I can not make up my mind about is whether to place the attribute lists below or above the Markdown blocks. The CommonMark people have discussed this for years. There is some context above, but I'm making this a simple "above" or "below":

{.myclass}
> foo
> bar

Or

> foo
> bar
{.myclass}


@HenrySkup
Copy link

Would it not make sense to follow the Commonmark and the Kramdown spec (i.e. after) to allow for everything to play well?

@bep Is this something that could be configurable?

@jmooring
Copy link
Member

jmooring commented Feb 8, 2021

Instead of above or below, think in terms of before or after. The existing implementation of heading attributes requires the attribute to be placed after the heading.

@bep
Copy link
Member Author

bep commented Feb 8, 2021

@bep Is this something that could be configurable?

Could, yes -- but ... that is a bad idea as it ties the content to the configuration making it very unportable.

The current implementation in this PR allows both before/after, but I also think that is a bad idea.

I think I agree with @jmooring -- and after is the closest we come to a "standard" at the moment.

@bep bep force-pushed the feat/goldmark-attrs branch from 18d28e0 to 786fd43 Compare February 8, 2021 17:47
E.g.:

```
> foo
> bar
{.myclass}
```

There are some current limitations: For tables you can currently only apply it to the full table, and for lists the ul/ol-nodes only, e.g.:

```
* Fruit
  * Apple
  * Orange
  * Banana
  {.fruits}
* Dairy
  * Milk
  * Cheese
  {.dairies}
{.list}
```

Fixes gohugoio#7548
@bep bep force-pushed the feat/goldmark-attrs branch from 786fd43 to ddd3a93 Compare February 8, 2021 18:00
@bep bep changed the title work in progress: markup/goldmark: Add attributes support for blocks (tables etc.) markup/goldmark: Add attributes support for blocks (tables etc.) Feb 8, 2021
@bep bep merged commit 2681633 into gohugoio:master Feb 8, 2021
@github-actions
Copy link

github-actions bot commented Feb 9, 2022

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 9, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ship goldmark-attributes extension
4 participants